fix(compose): allow overriding the IScopes used by SentryTraced via LocalSentryScopes - #5838
fix(compose): allow overriding the IScopes used by SentryTraced via LocalSentryScopes#5838Tabishahmad wants to merge 11 commits into
Conversation
…ocalSentryScopes SDKs that report their own telemetry through a separate IScopes/Hub instance (distinct from the host app's Sentry setup) previously had no way to make SentryTraced trace against that instance, since it always read from the global Sentry.getCurrentScopes(). LocalSentryScopes can now be overridden via CompositionLocalProvider to scope tracing to a specific IScopes, while sibling SentryTraced calls under the same scopes still share one root composition/render span. Fixes getsentry#2668
…an caching Key scopes weakly in RootSpans so custom IScopes instances (and their cached parent spans) can be garbage collected once nothing else holds a reference, instead of being pinned for the lifetime of the root Composition. Also stop permanently caching a null parent span when no transaction is bound yet, so a later transaction on the same scopes is still picked up. Addresses review feedback from sentry-review-bot and cursor bugbot on getsentry#5838. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…an caching Key scopes weakly in RootSpans so custom IScopes instances (and their cached parent spans) can be garbage collected once nothing else holds a reference, instead of being pinned for the lifetime of the root Composition. Also stop permanently caching a null parent span when no transaction is bound yet, so a later transaction on the same scopes is still picked up. Addresses review feedback from sentry-review-bot and cursor bugbot on getsentry#5838.
dbb7028 to
105fd6d
Compare
|
Thanks for the reviews! Addressed everything in 105fd6d:
Existing SentryTracedTest suite (sibling sharing + per-scopes isolation) still passes. |
Missing apiDump entry for the public LocalSentryScopes CompositionLocal was failing the apiCheck CI job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The cached ImmutableHolder<ISpan?> values strongly referenced their own IScopes key (via Span's internal scopes field), which kept every scopes instance permanently reachable through RootSpans and defeated the WeakHashMap keying entirely. Wrap the cached holder in a WeakReference so the map no longer holds a strong path back to its own key. LocalSentryScopes now defaults to null instead of eagerly resolving Sentry.getCurrentScopes(): a CompositionLocal's default factory runs at most once per process, so the previous default would permanently cache whatever scopes were current on first read (e.g. NoOp scopes if read before Sentry.init()). SentryTraced now falls back to Sentry.getCurrentScopes() on every call when nothing was explicitly provided. Also adds missing Sentry.close() teardown to SentryTracedTest, and fixes a typo in the changelog example. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r root spans Wrapping the cached root span holder in a WeakReference fixed the prior leak but let GC collect it between recompositions even while a SentryTraced call under that scopes was still mounted, silently breaking the "sibling calls share one root span" guarantee. RootSpans now uses plain HashMaps with reference counting: each SentryTraced call retains its scopes entry via a DisposableEffect and releases it on dispose. An entry is only removed once the last SentryTraced call using that IScopes actually leaves composition, so cleanup no longer depends on GC timing.
… dispose/remember race Retaining inside a DisposableEffect deferred the retain to the effect-application phase, but Compose dispatches an outgoing composable's onDispose before an incoming composable's DisposableEffect in the same recomposition. Replacing a SentryTraced call under a given scopes (e.g. during navigation) could therefore have the outgoing call's release clear the cache before the incoming call's retain ran, so a later SentryTraced call under the same scopes found nothing cached and created a duplicate root span. Retain now happens inside remember instead, which runs synchronously during composition, before any effect-phase work for that frame - including another SentryTraced call's dispose. Release stays in DisposableEffect's onDispose since its timing no longer matters. Adds a regression test that swaps which keyed SentryTraced composable is mounted under the same scopes twice in a row, which reproduces the duplicate span without this fix.
retain ran synchronously as a side effect of remember, while release only ran from DisposableEffect's onDispose. If the composition that retained never actually committed (e.g. an exception thrown elsewhere during the same composition), onDispose never fired, so the refcount was never decremented and the IScopes entry stayed pinned in RootSpans for the process lifetime. Replaced the plain remember + DisposableEffect pair with a single RememberObserver: retain happens in its constructor (still synchronous, so it keeps running before any effect-phase work for that frame), and release happens from either onForgotten (normal dispose) or onAbandoned (composition never committed), whichever Compose actually calls.
isIdle spans only auto-finish when the whole transaction finishes, so evicting a cached root span from RootSpans without finishing it left it open on the transaction. If the same scopes was used again later, a fresh root span was created alongside the still-open, now-untracked original, showing up as a duplicate root span once the transaction finished. release now finishes the evicted composition/rendering spans before dropping them from the cache.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 20dafa5. Configure here.
…activation Finishing spans when a RootSpans cache entry is evicted turned out to be unsafe: Compose can deactivate a composable's remembered state (onForgotten) and later reactivate the same instance (onRemembered) without re-running remember, e.g. for LazyColumn item reuse. A refcount reaching zero during a brief deactivation doesn't reliably mean the scopes is done for good, so eagerly finishing there risked silently dropping compose/render spans for content that was still in use, just temporarily deactivated. release no longer finishes evicted spans; they still get finished automatically once the whole transaction finishes, same as before this cache existed. The trade-off is a possible extra root span if the same scopes is genuinely reused much later. ScopesRetention also now tracks whether it currently holds a retain and re-retains from onRemembered when reactivated without the constructor re-running, instead of only retaining once at construction time. Without this, a reactivated call's claim on a shared cache entry was silently lost.

SDKs that report their own telemetry through a separate IScopes/Hub instance (distinct from the host app's Sentry setup) previously had no way to make SentryTraced trace against that instance, since it always read from the global Sentry.getCurrentScopes(). LocalSentryScopes can now be overridden via CompositionLocalProvider to scope tracing to a specific IScopes, while sibling SentryTraced calls under the same scopes still share one root composition/render span.
📜 Description
Adds a public
LocalSentryScopesCompositionLocal tosentry-compose, defaulting toSentry.getCurrentScopes()and overridable viaCompositionLocalProvider(LocalSentryScopes provides myScopes) { ... }.SentryTracednow resolves the currentIScopesfromLocalSentryScopesinstead of always reading the global default. The root "Initial Composition" / "Initial Render" spans are cached perIScopesin a shared holder so that siblingSentryTracedcalls under the same scopes still share one root span (matching current behavior), while calls under different scopes each get their own independent root span.💡 Motivation and Context
Enables SDKs/libraries that report their own telemetry through a separate
IScopes/Hubinstance to useSentryTracedagainst that instance, independently of the host app's own Sentry setup. Direction confirmed with @markushi and @romtsn on the issue.💚 How did you test it?
Added
SentryTracedTestcovering:IScopesprovided viaLocalSentryScopesSentryTracedcalls under the same scopes share one root spanSentryTracedcalls under different scopes don't interfere with each otherExisting
ComposeIntegrationTestsstill pass.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Update public API docs for
sentry-composeonce this lands, to documentLocalSentryScopes.